-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Print diagnostic messages on stderr, not stdout #1641
Print diagnostic messages on stderr, not stdout #1641
Conversation
Move some messages that are not strictly part of the "useful output" of the program to stderr, including warnings, errors, userconfirm prompts, and output related to key import.
And thanks for the great description with examples! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Marking as RFR. CI PR: rpm-software-management/ci-dnf-stack#1540. |
I've run the tests locally. It should be OK, there were few failures, but this should be due to different dnf5 bases here in the PR vs all the tests included in the CI PR. |
58cfa3f
We don't care that much where `dnf` puts the info about which packages were installed. Plus in different `dnf` versions the output goes into different output streams. See also: * rpm-software-management/dnf5#1641
We don't care that much where `dnf` puts the info about which packages were installed. Plus in different `dnf` versions the output goes into different output streams. See also: * rpm-software-management/dnf5#1641
We don't care that much where `dnf` puts the info about which packages were installed. Plus in different `dnf` versions the output goes into different output streams. See also: * rpm-software-management/dnf5#1641
We don't care that much where `dnf` puts the info about which packages were installed. Plus in different `dnf` versions the output goes into different output streams. See also: * rpm-software-management/dnf5#1641
We don't care that much where `dnf` puts the info about which packages were installed. Plus in different `dnf` versions the output goes into different output streams. See also: * rpm-software-management/dnf5#1641
We don't care that much where `dnf` puts the info about which packages were installed. Plus in different `dnf` versions the output goes into different output streams. See also: * rpm-software-management/dnf5#1641
Resolves #1361
This patchset changes many messages to be printed on stderr instead of stdout.
Opinions vary, but by general convention, the "useful output" of a program (that might be e.g. piped to a pager or consumed by a script), should be on stdout. All other messages, including warnings, progress bars, "Are you sure you want to continue?"-style prompts should be on stderr.
There are gray areas here, especially when you consider the same piece of output in different contexts. You could argue that the "Updating and loading repositories" message and multiprogressbar are "useful output" of the
makecache
command, but the same messages are not "useful output" of therepoquery
command. And DNF 5 commands such asinstall
andupgrade
are primarily "imperative" and don't have "useful output" in the same way that commands likerepolist
andrepoquery
do. So here, I've tried to use my best judgment and err on the side of keeping the status quo.The following messages are moved from stdout to stderr:
Context::print_info
go to stderr, which includes:To observe these changes, it's useful to redirect both streams to files:
and watch them from other terminals:
Output of
dnf5 --refresh rq rawhide xz
after this patch(before this patch, all output was on stdout.)
merged stdout and stderr
stdout
stderr
Output of
dnf5 install hello
after this patch(before this patch, all output was on stdout.)
merged stdout and stderr
stdout
stderr
I anticipate this PR needing a large ci-dnf-test patch, so I've marked it as a draft.